fix user creation using LDAP Plugin #12452

viniciuscb · 2018-11-14T14:16:03Z

When upgrading our installation to nextcloud14 we could not anymore create users in LDAP.

These are changes in user_ldap app that needed to be made to the createUser in the plugin work again.

We needed to change the createUser function in our plugin also, now it returns the user DN in LDAP, instead of returning a boolean. Other developers who created a plugin for LDAP will need to change this also.

Their code will break anyway when they try to upgrade to nc14, so we are now throwing an exception when the custom createUser function returns true.

Here is our plugin code:
https://gitlab.com/eita/rios/user_ldap_extended/tree/nc14

Signed-off-by: Arthur Schiwon <[email protected]>

blizzz · 2019-02-14T11:58:51Z

@viniciuscb sorry, I missed this PR. Ideally, next time ping the @nextcloud/ldap group, to ensure it does not get buried.

apps/user_ldap/lib/User_LDAP.php

viniciuscb · 2019-02-19T21:36:26Z

@viniciuscb sorry, I missed this PR. Ideally, next time ping the @nextcloud/ldap group, to ensure it does not get buried.

Hi @blizzz I will take a look and test it in master.

ChristophWurst · 2019-03-01T08:46:12Z

Any updates, @viniciuscb? The feature freeze for Nextcloud 16 is today, hence I'm afraid we have to move this to 17.

blizzz · 2019-03-01T09:29:26Z

@ChristophWurst it's a bug fix, therefore the freeze does not apply here

Signed-off-by: Vinicius Cubas Brand <[email protected]>

viniciuscb · 2019-03-01T16:33:10Z

@blizzz @ChristophWurst took a review according to @blizzz recommendations, tested in local (creating a ldap user / group), have run the automated tests, all worked.

This commit fix an error happening when the subadmin tries to create an user, adding him/her to the group s/he is subadmin of, using a LDAP User/Group plugin. This just forces the cache to be reset after an user is added to a group. Signed-off-by: Vinicius Cubas Brand <[email protected]>

blizzz · 2019-03-05T00:17:53Z

I plan to review tomorrow

blizzz

Just one remark, good otherwise!

apps/user_ldap/lib/User_LDAP.php

MorrisJobke

Code makes sense 👍

LDAP plugins must change the createUser method to return the DN, as we need this to update the cache. Signed-off-by: Vinicius Cubas Brand <[email protected]>

MorrisJobke · 2019-03-14T15:28:49Z

/backport to stable15

MorrisJobke · 2019-03-14T15:28:52Z

/backport to stable14

MorrisJobke · 2019-03-14T19:30:12Z

Status of 17004: failure

ENABLE_OPENLDAP=true, ENABLE_REDIS=true, TESTS=integration-ldap-openldap-features

build/integration/ldap_features/ldap-openldap.feature:106
build/integration/ldap_features/ldap-openldap.feature:127
build/integration/ldap_features/ldap-openldap.feature:148

Show full log

  Scenario: Test LDAP group membership with intermediate groups not matching filter # /drone/src/github.com/nextcloud/server/build/integration/ldap_features/ldap-openldap.feature:106
[Thu Mar 14 18:07:00 2019] 127.0.0.1:57172 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config
[Thu Mar 14 18:07:00 2019] 127.0.0.1:57176 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:00 2019] 127.0.0.1:57182 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
    Given modify LDAP configuration                                                 # LDAPContext::modifyLDAPConfiguration()
      | ldapBaseGroups                | ou=OtherGroups,dc=nextcloud,dc=ci            |
      | ldapGroupFilter               | (&(cn=Gardeners)(objectclass=groupOfNames))  |
      | ldapNestedGroups              | 1                                            |
      | useMemberOfToDetectMembership | 1                                            |
      | ldapUserFilter                | (&(objectclass=inetorgperson)(!(uid=alice))) |
      | ldapExpertUsernameAttr        | uid                                          |
      | ldapGroupMemberAssocAttr      | member                                       |
    And As an "admin"                                                               # LDAPContext::asAn()
[Thu Mar 14 18:07:00 2019] 127.0.0.1:57194 [200]: /ocs/v2.php/cloud/groups
    And sending "GET" to "/cloud/groups"                                            # LDAPContext::sendingTo()
[Thu Mar 14 18:07:00 2019] 127.0.0.1:57206 [404]: /ocs/v2.php/cloud/groups/Gardeners/users
    And sending "GET" to "/cloud/groups/Gardeners/users"                            # LDAPContext::sendingTo()
    Then the OCS status code should be "200"                                        # LDAPContext::theOCSStatusCodeShouldBe()
      Failed asserting that SimpleXMLElement Object &000000006275076400000000279c7e09 (
          0 => '404'
      ) matches expected '200'.
    And the "users" result should match                                             # LDAPContext::theGroupResultShouldMatch()
      | alice  | 0 |
      | clara  | 1 |
      | elisa  | 1 |
      | gustaf | 1 |
      | jesper | 1 |
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57224 [200]: /apps/testing/clean_opcode_cache.php
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57226 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01

  Scenario: Test LDAP group membership with intermediate groups not matching filter and without memberof # /drone/src/github.com/nextcloud/server/build/integration/ldap_features/ldap-openldap.feature:127
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57244 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57258 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57266 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
    Given modify LDAP configuration                                                                      # LDAPContext::modifyLDAPConfiguration()
      | ldapBaseGroups                | ou=OtherGroups,dc=nextcloud,dc=ci            |
      | ldapGroupFilter               | (&(cn=Gardeners)(objectclass=groupOfNames))  |
      | ldapNestedGroups              | 1                                            |
      | useMemberOfToDetectMembership | 0                                            |
      | ldapUserFilter                | (&(objectclass=inetorgperson)(!(uid=alice))) |
      | ldapExpertUsernameAttr        | uid                                          |
      | ldapGroupMemberAssocAttr      | member                                       |
    And As an "admin"                                                                                    # LDAPContext::asAn()
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57284 [200]: /ocs/v2.php/cloud/groups
    And sending "GET" to "/cloud/groups"                                                                 # LDAPContext::sendingTo()
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57300 [404]: /ocs/v2.php/cloud/groups/Gardeners/users
    And sending "GET" to "/cloud/groups/Gardeners/users"                                                 # LDAPContext::sendingTo()
    Then the OCS status code should be "200"                                                             # LDAPContext::theOCSStatusCodeShouldBe()
      Failed asserting that SimpleXMLElement Object &000000006275072d00000000279c7e09 (
          0 => '404'
      ) matches expected '200'.
    And the "users" result should match                                                                  # LDAPContext::theGroupResultShouldMatch()
      | alice  | 0 |
      | clara  | 1 |
      | elisa  | 1 |
      | gustaf | 1 |
      | jesper | 1 |
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57324 [200]: /apps/testing/clean_opcode_cache.php
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57326 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01

  Scenario: Test LDAP group membership with intermediate groups not matching filter, numeric group ids # /drone/src/github.com/nextcloud/server/build/integration/ldap_features/ldap-openldap.feature:148
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57338 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57346 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57364 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
    Given modify LDAP configuration                                                                    # LDAPContext::modifyLDAPConfiguration()
      | ldapBaseGroups                | ou=NumericGroups,dc=nextcloud,dc=ci          |
      | ldapGroupFilter               | (&(cn=2000)(objectclass=groupOfNames))       |
      | ldapNestedGroups              | 1                                            |
      | useMemberOfToDetectMembership | 1                                            |
      | ldapUserFilter                | (&(objectclass=inetorgperson)(!(uid=alice))) |
      | ldapExpertUsernameAttr        | uid                                          |
      | ldapGroupMemberAssocAttr      | member                                       |
    And As an "admin"                                                                                  # LDAPContext::asAn()
[Thu Mar 14 18:07:02 2019] ldap_control_paged_result_response(): Result is: No such object (32) at /drone/src/github.com/nextcloud/server/apps/user_ldap/lib/LDAP.php#74
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57380 [200]: /ocs/v2.php/cloud/groups
    And sending "GET" to "/cloud/groups"                                                               # LDAPContext::sendingTo()
[Thu Mar 14 18:07:03 2019] 127.0.0.1:57392 [404]: /ocs/v2.php/cloud/groups/2000/users
    And sending "GET" to "/cloud/groups/2000/users"                                                    # LDAPContext::sendingTo()
    Then the OCS status code should be "200"                                                           # LDAPContext::theOCSStatusCodeShouldBe()
      Failed asserting that SimpleXMLElement Object &00000000627507fa00000000279c7e09 (
          0 => '404'
      ) matches expected '200'.
    And the "users" result should match                                                                # LDAPContext::theGroupResultShouldMatch()
      | alice  | 0 |
      | clara  | 1 |
      | elisa  | 1 |
      | gustaf | 1 |
      | jesper | 1 |
[Thu Mar 14 18:07:03 2019] 127.0.0.1:57426 [200]: /apps/testing/clean_opcode_cache.php
[Thu Mar 14 18:07:04 2019] 127.0.0.1:57428 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01

ENABLE_OPENLDAP=true, ENABLE_REDIS=true, TESTS=integration-ldap-openldap-numerical-id-features

build/integration/ldap_features/openldap-numerical-id.feature:33
build/integration/ldap_features/openldap-numerical-id.feature:49

Show full log

  Scenario: Test LDAP group retrieval with numeric group ids and nesting # /drone/src/github.com/nextcloud/server/build/integration/ldap_features/openldap-numerical-id.feature:33
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57232 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57238 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57250 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57270 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
    Given modify LDAP configuration                                      # LDAPContext::modifyLDAPConfiguration()
      | ldapBaseGroups                | ou=NumericGroups,dc=nextcloud,dc=ci |
      | ldapGroupFilter               | (objectclass=groupOfNames)          |
      | ldapNestedGroups              | 1                                   |
      | useMemberOfToDetectMembership | 1                                   |
    And As an "admin"                                                    # LDAPContext::asAn()
[Thu Mar 14 18:07:01 2019] ldap_control_paged_result_response(): Result is: No such object (32) at /drone/src/github.com/nextcloud/server/apps/user_ldap/lib/LDAP.php#74
[Thu Mar 14 18:07:01 2019] 127.0.0.1:57288 [200]: /ocs/v2.php/cloud/groups
    And sending "GET" to "/cloud/groups"                                 # LDAPContext::sendingTo()
    Then the OCS status code should be "200"                             # LDAPContext::theOCSStatusCodeShouldBe()
    And the "groups" result should match                                 # LDAPContext::theGroupResultShouldMatch()
      | 2000 | 1 |
      | 3000 | 1 |
      | 3001 | 1 |
      | 3002 | 1 |
      Failed asserting that an array contains '2000'.
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57318 [200]: /apps/testing/clean_opcode_cache.php
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57320 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01

  Scenario: Test LDAP group membership with intermediate groups not matching filter, numeric group ids # /drone/src/github.com/nextcloud/server/build/integration/ldap_features/openldap-numerical-id.feature:49
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57342 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57348 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57360 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
[Thu Mar 14 18:07:02 2019] 127.0.0.1:57376 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01
    Given modify LDAP configuration                                                                    # LDAPContext::modifyLDAPConfiguration()
      | ldapBaseGroups                | ou=NumericGroups,dc=nextcloud,dc=ci          |
      | ldapGroupFilter               | (&(cn=2000)(objectclass=groupOfNames))       |
      | ldapNestedGroups              | 1                                            |
      | useMemberOfToDetectMembership | 1                                            |
      | ldapUserFilter                | (&(objectclass=inetorgperson)(!(uid=alice))) |
      | ldapGroupMemberAssocAttr      | member                                       |
    And As an "admin"                                                                                  # LDAPContext::asAn()
[Thu Mar 14 18:07:03 2019] ldap_control_paged_result_response(): Result is: No such object (32) at /drone/src/github.com/nextcloud/server/apps/user_ldap/lib/LDAP.php#74
[Thu Mar 14 18:07:03 2019] 127.0.0.1:57388 [200]: /ocs/v2.php/cloud/groups
    And sending "GET" to "/cloud/groups"                                                               # LDAPContext::sendingTo()
[Thu Mar 14 18:07:03 2019] 127.0.0.1:57398 [404]: /ocs/v2.php/cloud/groups/2000/users
    And sending "GET" to "/cloud/groups/2000/users"                                                    # LDAPContext::sendingTo()
    Then the OCS status code should be "200"                                                           # LDAPContext::theOCSStatusCodeShouldBe()
      Failed asserting that SimpleXMLElement Object &000000007a7b6834000000002316ece3 (
          0 => '404'
      ) matches expected '200'.
    And the "users" result should match                                                                # LDAPContext::theGroupResultShouldMatch()
      | 92379 | 0 |
      | 54172 | 1 |
      | 50194 | 1 |
      | 59376 | 1 |
      | 59463 | 1 |
[Thu Mar 14 18:07:03 2019] 127.0.0.1:57432 [200]: /apps/testing/clean_opcode_cache.php
[Thu Mar 14 18:07:03 2019] 127.0.0.1:57434 [200]: /ocs/v2.php/apps/user_ldap/api/v1/config/s01

MorrisJobke · 2019-03-14T19:31:03Z

@viniciuscb @blizzz Looks like there is something wrong with the tests :/

blizzz · 2019-03-14T21:39:10Z

rebase might solve it

MorrisJobke · 2019-03-21T09:22:14Z

Rebased on master: #14778

viniciuscb referenced this pull request Nov 14, 2018

adjust and add more unit tests

846ab25

Signed-off-by: Arthur Schiwon <[email protected]>

viniciuscb force-pushed the user_ldap_createuser_fix branch from 53180dc to 3d332c3 Compare November 14, 2018 14:28

blizzz requested changes Feb 14, 2019

View reviewed changes

apps/user_ldap/lib/User_LDAP.php Outdated Show resolved Hide resolved

apps/user_ldap/lib/User_LDAP.php Outdated Show resolved Hide resolved

blizzz added bug 2. developing Work in progress feature: ldap labels Feb 14, 2019

blizzz added this to the Nextcloud 16 milestone Feb 14, 2019

blizzz added the backport-request label Feb 14, 2019

viniciuscb force-pushed the user_ldap_createuser_fix branch from 3d332c3 to 9a4b5c9 Compare March 1, 2019 15:37

fix user creation using LDAP Plugin

62ab9e0

Signed-off-by: Vinicius Cubas Brand <[email protected]>

viniciuscb force-pushed the user_ldap_createuser_fix branch from 9a4b5c9 to 62ab9e0 Compare March 1, 2019 16:04

MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 4, 2019

MorrisJobke mentioned this pull request Mar 4, 2019

16.0.0 Alpha 1 #14501

Merged

45 tasks

blizzz reviewed Mar 5, 2019

View reviewed changes

apps/user_ldap/lib/User_LDAP.php Outdated Show resolved Hide resolved

MorrisJobke approved these changes Mar 5, 2019

View reviewed changes

MorrisJobke mentioned this pull request Mar 6, 2019

16.0.0 Beta 1 #14573

Merged

9 tasks

LDAP plugin: force createUser to return new user's DN

e69696c

LDAP plugins must change the createUser method to return the DN, as we need this to update the cache. Signed-off-by: Vinicius Cubas Brand <[email protected]>

blizzz approved these changes Mar 14, 2019

View reviewed changes

blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 14, 2019

This comment has been minimized.

Sign in to view

MorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Mar 14, 2019

MorrisJobke mentioned this pull request Mar 21, 2019

Fix user creation using LDAP Plugin #14778

Merged

MorrisJobke closed this Mar 21, 2019

MorrisJobke removed the backport-request label Mar 21, 2019

Uh oh!

fix user creation using LDAP Plugin #12452

fix user creation using LDAP Plugin #12452

Uh oh!

Conversation

viniciuscb commented Nov 14, 2018

Uh oh!

blizzz commented Feb 14, 2019

Uh oh!

Uh oh!

Uh oh!

viniciuscb commented Feb 19, 2019

Uh oh!

ChristophWurst commented Mar 1, 2019

Uh oh!

blizzz commented Mar 1, 2019

Uh oh!

viniciuscb commented Mar 1, 2019

Uh oh!

blizzz commented Mar 5, 2019

Uh oh!

blizzz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

MorrisJobke commented Mar 14, 2019

Uh oh!

MorrisJobke commented Mar 14, 2019

Uh oh!

MorrisJobke commented Mar 14, 2019

Status of 17004: failure

ENABLE_OPENLDAP=true, ENABLE_REDIS=true, TESTS=integration-ldap-openldap-features

ENABLE_OPENLDAP=true, ENABLE_REDIS=true, TESTS=integration-ldap-openldap-numerical-id-features

Uh oh!

MorrisJobke commented Mar 14, 2019

Uh oh!

blizzz commented Mar 14, 2019

Uh oh!

MorrisJobke commented Mar 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants